Fix steam.exe crash on driver unload (0xc0000005) - fixes #3205#3333
Conversation
There was a problem hiding this comment.
Thank you very much for your work. I followed through your explanation and the code and it makes sense. The implementation is clean, I would not have done things differently.
There is only a consistency nit to address. initialize_runtime() is actually a C++ -> Rust call. we already have a bunch of those but we manage them differently, we assign the function pointers on the Rust side, which now are actually under initialize_runtime() itself. I wonder if all (or most) of those function pointers could be initialized how you did (just expose them from Rust with #[unsafe(no_mangle)] external "C"). So I'd ask if you can do that refactoring also.
Another small nit is the casing of the function, which should follow pascal case: InitializeRuntime
|
Will there be a new v20 patch release with this fix? |
Addressing both points and working on it now! I'm going with |
6696af4 to
555ed3c
Compare
|
Done and pushed. All the function pointers are now direct exports, the assignments and C++ pointer definitions are gone, and no call sites needed changes. Side effect: the exports are valid from DLL load, so the old null-pointer-before-init hazard is gone. Compiles on master, and I runtime tested the same changes backported to my v20.14.1 build with a full SteamVR session + vrchat |
04c1a96 to
cca47c6
Compare
cca47c6 to
518704e
Compare
zmerp
left a comment
zmerp
left a comment
There was a problem hiding this comment.
Perfect! thank you :) I'll wait one night (in case if something pops up) then I'll merge
|
@satherton Probably not, but you can use nightly |
3 participants
steam.exe crashes with an access violation (0xc0000005) in driver_alvr_server.dll_unloaded. Reproduces reliably here: start SteamVR through Steam, then quit. steam.exe crashes on exit.
Steam enumerates VR drivers by loading the DLL, calling HmdDriverFactory, then unloading it with FreeLibrary. The factory currently runs the whole driver init on first call, so a simple probe spawns threads, shutdown_driver() never runs in that process, and the threads are still alive when the DLL gets unmapped. They end up executing freed memory.
WinDbg on the minidumps with local PDBs showed which threads were dying: first a rust thread mid teardown (thread_start into __rust_dealloc), then rayon workers (WorkerThread::find_work, Sleep::sleep). The rayon pool comes from sysinfo::System::new_all() in the factory, sysinfo's default multithread feature spins it up and it's never joined.
The fix: heavy init moved out of the factory into a new initialize_runtime() called from DriverProvider::Init(), so a probe spawns nothing and init only happens when vrserver actually activates the driver. shutdown_driver() now joins the event loop and idle init threads. And sysinfo's multithread feature is disabled in server_openvr and server_core to drop the rayon pool.
Verified on v20.14.1, the crash hit every SteamVR session before and is gone after. This branch is the same fix rebased onto master and it compiles.
Same crash as #3331 but fixed by removing the cause instead of special casing steam.exe. Valve's own driver_vrlink.dll crashes the same way in steam.exe, which fits the probe and unload mechanism.